-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci/nixpkgs-vet: 0.1.2 -> 0.1.4 #339119
ci/nixpkgs-vet: 0.1.2 -> 0.1.4 #339119
Conversation
258e58e
to
d58c2a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in PMs, probably best not to add pkgs.nixpkgs-vet
:
- It would be confusing because it's not the actual version of the tool that should be used to check CI. Instead
./maintainers/scripts/nixpkgs-vet.sh
should be used, which fetches the correct version (though it also needs to build it) - CI won't be able to use the version from Nixpkgs, nor reuse its Hydra build results
- We'd be duplicating the Nix expression here, they could easily drift apart (and indeed did already here!)
- Nixpkgs-vet should be considered downstream from Nixpkgs. With the automated weekly update action we have in the Nixpkgs-vet repo, we'll be notified if any dependency breaks the build. I don't think there's a need for package maintainers to try to prevent build failures in Nixpkgs already.
- It's also just less confusing, less recursive references :)
Adding libSrc
for the other purposes sounds fine though
Other than that and the couple comments, I think this looks good!
d58c2a4
to
9bece8f
Compare
Dropped commits making |
Ohh, we should totally move |
I don't see any reason to make a new symlink in |
Sounds good to me! |
f12a88b
to
2f3c8b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, thanks!
2f3c8b0
to
841d873
Compare
Everything gets moved into the `ci/` top-level directory. We keep behind `maintainers/scripts/check-by-name.sh` and `pkgs/test/check-by-name/pinned-version.txt` as they are going to cause CI errors and confusion until we get all the way through the various channels. They'll be removed in about a week or so.
841d873
to
89cbfde
Compare
OK, I finished futzing with |
Should we backport this to 24.05? I think we should. |
# | ||
# When you make changes to this workflow, also update pkgs/test/check-by-name/run-local.sh adequately | ||
# Checks pkgs/by-name (see pkgs/by-name/README.md) using the `nixpkgs-vet` tool (see https://github.com/NixOS/nixpkgs-vet) | ||
# When you make changes to this workflow, please also update `ci/nixpkgs-vet.sh` to reflect the impact of your work to the CI. | ||
name: Check pkgs/by-name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh just noticed that this would also be good to change
I don't think we need to bother backporting, because anything that only goes to release branches won't affect Nixpkgs long-term |
I guess it might be confusing if different checks are enforced for different PRs though |
Motivation for change
nixpkgs-vet
nixpkgs-vet#100The
nixpkgs-check-by-name
tool is expanding its scope to vet more changes across Nixpkgs.Description of changes
nixpkgs-check-by-name
tonixpkgs-vet
.Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)